-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rustc_codegen_llvm: use safe references for LLVM FFI types. #52461
Conversation
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
cc @rust-lang/compiler |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors r+ p=1 GIving p=1 because this feels like the kind of thing that bitrots quickly. |
📌 Commit 7c1c1b42a9a3b901137b07b78cef821f4444b944 has been approved by |
I wrote up a quick documentation of the technique used in this PR: #43467 (comment). |
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
☔ The latest upstream changes (presumably #52353) made this pull request unmergeable. Please resolve the merge conflicts. |
@bors r=nikomatsakis |
📌 Commit be90793c06a95affbc31133e3c23c761e6515b86 has been approved by |
⌛ Testing commit be90793c06a95affbc31133e3c23c761e6515b86 with merge e1c781383d7f766bf17e5a2e50a87a58be9a581a... |
💔 Test failed - status-appveyor |
@alexcrichton Ugh do we need EDIT: oh, maybe it wasn't needed for |
@bors r=nikomatsakis |
📌 Commit 69ed6b9 has been approved by |
⌛ Testing commit 69ed6b9 with merge 3140e880e5d232b710d23fc92911b4e0879869c4... |
💔 Test failed - status-appveyor |
@bors r- This PR still appears scheduled 😕. Panicked on
|
Alright so it's not just @irinagpopa hitting that locally, there's an actual stage0 bug being triggered. IIRC there's no debuginfo and |
Turns out that there's a bad unwrap (and one more above it): rust/src/librustc_metadata/native_libs.rs Lines 174 to 176 in 8961132
Those should be matching on span like neighboring code. (cc @alexcrichton @Mark-Simulacrum)
@bors r=nikomatsakis |
📌 Commit baff67d has been approved by |
rustc_codegen_llvm: use safe references for LLVM FFI types. Part of #45274.
☀️ Test successful - status-appveyor, status-travis |
rustc_codegen_llvm: traitification of LLVM-specific CodegenCx and Builder methods This PR is the continuation of #52461 in the grand plan of #45274 to allow for multiple codegen backends. A first attempt at this was #52987 but since @irinagpopa is no longer working on it I'm taking ownership of the PR. The changes are refactoring only and do not affect the logic of the code. Performance should not be impacted since all parametrization is done with generics (no trait objects). The `librustc_codegen_llvm` crate now contains a new folder `interfaces` that describes with traits part of how the compiler interfaces with LLVM during codegen. `CodegenCx` and `Builder` implement those traits. Many things are still missing. All the calls to LLVM are not yet under a trait, and later LLVM-agnostic code should be parametrized.
Part of #45274.